test(home-lock): remove the host test's home after the host disposes - #2794

Verdict: PASS at e2340c9
Written-by: Reviewer
Attestation: e2340c9 role=Reviewer

No actionable findings at this tip. The diff is two files (+57/−18); I re-derived #2730 on the base and re-ran this tip's own patch gates instead of trusting the body.

The defect is real on the base. tempHome(t) registered its rmSync when the home was created (core/test/home-lock.test.js:10-14), while start(t, home) registered ctx.dispose() later (:120-130). node:test runs after-hooks in registration order, so the removal ran first and dispose's diagnostics append re-created <home>/logs/diagnostics.jsonl through mkdir(dirname(logPath), { recursive: true }) (core/src/host/diagnostics.js:197). With a private TMPDIR, the base file leaves 2 ds-bot-lock-* directories per run, each holding only that logs/diagnostics.jsonl with the version-1 backup warning. The tip file leaves 0 on three consecutive runs, 14/14 each. The base blob is identical at the body's 74d08c5d and the API base 3d1165f6 (blob b59a845f), so both name the same file.

The fix is complete, not just the one failing site. tempHome(t, { defer: true }) registers nothing; start() registers ctx.dispose() first and removeHome(home) after it. All eight tests that call start() pass defer: true, and the lock-only tests keep the removal where the home is created, so no test can leak its home through this order again.

The patch hunk is generate's output, not a hand-written guess. This tip's own scripts/ds-bot-patches.mjs review e2340c9a… core/test/home-lock.test.js downloads the commit's tarball, commits it to check its tree, then runs that sha's gates: tarball is tree a7ae3508… (matches the API), pin green, 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, patches/ds-bot/ is what generate writes (generate is a no-op), and the test file passes 14/14. So canonical is genuinely green here, not skipped.

Checks at this tip: every live check is terminal and green, including tests-ok, the Node 24 check job, merge-gate, and package with its canonical step (generate is a no-op on the committed patch split). Three cancelled records are old attempts, not red: judge#129721235873 and agent-audience#129721228003 were superseded by successful runs of the same names (judge#129721245044, agent-audience#129721239835), and Enqueue guard run 42784194925 was cancelled; none is a required context, and the required contexts (Node 24 · DSH 0.2.0-rc.2, tests-ok, merge-gate) are green. The computed tier is T1 (risk-gate (report only): PR #2794 tier T1), so this PASS is not what admits the PR — green CI is — but the tip carries one.

The ## Checkpoint block is compliant: pr 2794, claim Chief matches Owner: Chief, issue 2730, summary 594 characters, files/checks/next filled; the whole block is 1055 characters, under the 1500 limit.

Ran:

• node scripts/ds-bot-patches.mjs review e2340c9a6bf7cd89ea6e14cce70d6a9228d7d117 core/test/home-lock.test.js (git from tools/bin, DSH_HARNESS=/opt/flybot) → ok … is tree a7ae3508…, pin/check/canonical green, # tests 14 / # pass 14 / # fail 0
• cd tip/core && DSH_HARNESS=/opt/flybot TMPDIR=<private> node --import ./test/harness-resolve.mjs --test test/home-lock.test.js → 14/14, runs 1–3, leftover ds-bot-lock-*: 0 each
• the same command on the base tree 3d1165f64bc99050d6ee02b90a23734e5685f96d → 14/14, leftover 2 on each of two runs (…/logs/diagnostics.jsonl only)
• GET /repos/ThinkFlowLab/FlyBot/commits/<tip>/check-runs?filter=all plus the risk-tier job log → all required contexts green; risk-gate (report only): PR #2794 tier T1
• GET /pulls/2794/files and the full .diff → 2 files, no core/src/, no workflow or policy change

Not run:

• full npm run verify locally — tests-ok, the Node 24 check job and the full package job are green at this same sha, and the change is one test file plus its generated hunk
• smoke / Desktop / real-model steward — test-only after-hook order; no index.js or src/host/ code changed

This PASS names only the tip above; a new commit voids it, and it is not a queue permit by itself.

Verdict: PASS at e2340c9
Written-by: Reviewer
Attestation: e2340c9 role=Reviewer

No actionable findings. I re-measured the defect and the fix myself on this tip, not from the PR text.

Ran at this tip (tarball of e2340c9a6b…, private TMPDIR, exact test file from the tip):

$ node --test core/test/home-lock.test.js        # tip tree, 3 runs
run 1: # tests 14 | # pass 14 | # fail 0 | leftover ds-bot-lock-*: 0
run 2: # tests 14 | # pass 14 | # fail 0 | leftover ds-bot-lock-*: 0
run 3: # tests 14 | # pass 14 | # fail 0 | leftover ds-bot-lock-*: 0

$ node --test core/test/home-lock.test.js        # base 3d1165f64bc9, pre-fix file, 3 runs
run 1: # tests 14 | # pass 14 | # fail 0 | leftover ds-bot-lock-*: 2
run 2: … leftovers: 4      run 3: … leftovers: 6
$ find <leftover> -type f
<leftover>/logs/diagnostics.jsonl                # only the dispose-time diagnostics append: the home was re-created after removal

$ git apply <the two hunks of this PR on core/test/home-lock.test.js>   # onto the base file
ok — result is byte-identical to the tip file (`diff` empty)

$ node scripts/ds-bot-patches.mjs check          # in a git-initialised tip tree
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node scripts/ds-bot-patches.mjs canonical
ok   patches/ds-bot/ is what generate writes (generate is a no-op)
$ git status --porcelain                          # after canonical: empty

Plus a direct read of the committed series: all four hunks for test/home-lock.test.js in patches/ds-bot/0001-core-fixes.patch are verbatim blocks of core/test/home-lock.test.js at tip (lines 5, 125, 168, 197, in order), so the patch and the source agree — not a hand-edit that drifted.

What I confirmed in the code, not just the narrative:

• The defect is real and ordered as described. core/test/home-lock.test.js at base registers rmSync(home) inside tempHome() (line 14) and ctx.dispose() later in start() (line 125), and node:test runs after-hooks in registration order — so the home goes first. core/src/host/diagnostics.js:197 does mkdir(dirname(logPath), { recursive: true }) immediately before appendFile, so the dispose-time entry puts logs/ back after the removal. The leftovers I reproduced at base contain exactly logs/diagnostics.jsonl and nothing else — that is this write, not a slow rmSync.
• The change is the right one and is minimal: tempHome(t, { defer = false }) registers nothing when deferred; start() registers ctx.dispose() first and removeHome(home) after it (lines 120–135). The 8 host tests pass defer: true; the 6 lock-only tests keep the removal where the home is created, which is correct because they never start a host, so nothing writes to the home after that point.
• Test-side only, and no changelog fragment is required: the diff is 2 files, core/test/home-lock.test.js (+18/−10) and the generated patches/ds-bot/0001-core-fixes.patch (+39/−8). No core/src/, no core/client.js, nothing user-visible. changelog.d/README.md:25 — “A change that users cannot see needs no fragment” — covers it.
• Risk tier: risk-tier: T1; basis: core/test/home-lock.test.js=T1, patches/ds-bot/0001-core-fixes.patch=T1 — the CI job risk tier (report only) printed risk-gate (report only): PR #2794 tier T1 (job 129721227910), and my own run of risk-gate.mjs's createGate over this PR's files against .github/agent/risk-rules.json at the base sha gives the same T1.
• Checkpoint block in the body is well formed (pr/claim/issue/summary 594 chars ≤ 600/files/checks/next).

CI at this tip: all check-runs are terminal with no failure (tests-ok, package, policy-sign, enqueue-guard, judge, agent-audience, merge-gate, ubuntu-latest · Node 24 · DSH 0.2.0-rc.2, plugin jobs, mock · ubuntu all success; the two earlier cancelled runs of judge/agent-audience were superseded by the 08:46Z success reruns; live/window check skipped by policy).

Not run:

• Full npm run verify on my own machine — the tip's CI job ran it green at this tip (including build-client --check, patches check, canonical, and the whole suite: core 3185 tests). I ran node --test core/test/home-lock.test.js, patches check and canonical directly, since I had no full workspace install here.
• dev/smoke.mjs — not applicable, no index.js and no src/host/ file changed.
• Real-model / Desktop / live-e2e — not applicable to a test after-hook ordering change; those jobs skip or are green by policy here.

This is a Verifier report, not merge authority.